Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: added auto detect content type #7236

Conversation

0x-chaitu
Copy link

Closes #6584

Change Description

Background

As from previous discussions detect-content-type and controllable auto detect , user can add a detect-content-flag.

Bug Fix

If this PR is a bug fix, please let us know about:

Problem - lakectl local didnt support --detect-content-type flag.
Solution - added user controllable detect-content-type support.

Testing Details

How were the changes tested?

They were tested by running lakectl local to upload files with content-type flag

Contact Details

[email protected])

@CLAassistant
Copy link

CLAassistant commented Jan 3, 2024

CLA assistant check
All committers have signed the CLA.

@0x-chaitu 0x-chaitu force-pushed the fix/lakectl-local-auto-detect-content-type branch from 68afd48 to 95e433e Compare January 3, 2024 16:05
Copy link

github-actions bot commented Feb 3, 2024

This PR is now marked as stale after 30 days of inactivity, and will be closed soon. To keep it, mark it with the "no stale" label.

@github-actions github-actions bot added the stale label Feb 3, 2024
Copy link

Closing this PR because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this Feb 11, 2024
@N-o-Z N-o-Z removed the stale label Jun 16, 2024
@N-o-Z N-o-Z reopened this Jun 16, 2024
@arielshaqed
Copy link
Contributor

Hi @0x-chaitu , @N-o-Z ,

I am closing this PR, but not through any fault of the code. There are numerous interactions between lakectl local and content types, and also between lakectl fs upload and content types. The underlying issue #6584 needs significant product guidance. We have had 2 previous attempts to resolve it, both failed due to lack of clarity of requirements. I believe that very partial content-type support will only confuse lakectl users.

Among the questions that cannot be addressed in a PR:

  • How do lakectl local and, separately, lakectl fs upload preserve existing content type? (Do they?)
  • How do we prevent lakectl local and, separately, lakectl fs upload, from clobbering content types? Note that something like a ".HTM" extension can actually have multiple content-types.
  • How do we support content-type in web UI uploads?
  • How do we maintain backwards compatibility through all of this?

So unfortunately this PR cannot be accepted in its current form. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Currently lakectl local doesn't support upload with content type.
4 participants